Conversation
dougwilson
left a comment
dougwilson
left a comment
There was a problem hiding this comment.
Thanks! I added a few comments and I also tagged it in that documentation should be added to the README so users will know how to enable the debugging and what the different messages mean and how to use it to diagnose their cors issues. You can always wait on the docs until the code is settled as well if you like.
|
|
||
| var assign = require('object-assign'); | ||
| var vary = require('vary'); | ||
| var debug = require('debug')('express:cors'); |
There was a problem hiding this comment.
Ideally this should just be the name of the npm module: cors.
| "repository": "expressjs/cors", | ||
| "main": "./lib/index.js", | ||
| "dependencies": { | ||
| "debug": "^4.3.1", |
There was a problem hiding this comment.
I would set this to the same version express uses, or at least one compatible with the version range of this module.
| } | ||
|
|
||
| function isOriginAllowed(origin, allowedOrigin) { | ||
| debug('test of origin %s. AllowedOrigin: %O', origin, allowedOrigin); |
There was a problem hiding this comment.
You can likely simply this entire block by creating a new variable to store the true/false and then have a single debug call at the end with what was checked and the result. This would also fix the issue here where given an array, it will never log isAllowedOrigin false.
|
|
||
| if (!options.origin || options.origin === '*') { | ||
| // allow any origin | ||
| debug('added header Access-Control-Allow-Origin *'); |
There was a problem hiding this comment.
Rather than duplicating this on every single header add line, you could just place a single call when the headers are populated to list them.
2 participants
Closes #233
This is a PR to add a debug to middleware cors.